Change server information endpoint / to only accept GET and HEAD requests
#15976
Conversation
|
Warning It looks like this PR modifies one or more |
|
This pull request does not have a backport label. Could you fix it @carsonip? 🙏
|
carsonip
added
the
backport-active-9
carsonip
changed the title
/ to only accept GET and HEAD method
|
Warning It looks like this PR modifies one or more |
internal/beater/api/root/handler.go
Outdated
| c.Result.SetWithError( | ||
| request.IDResponseErrorsMethodNotAllowed, | ||
| // include a verbose error message to alert users about a common misconfiguration | ||
| errors.New("this is the health check endpoint; did you mean to send data to another endpoint instead?"), |
There was a problem hiding this comment.
[to reviewer] lmk if this is too verbose, I can revert the commit.
$ curl -XPOST -v localhost:8200/
* Host localhost:8200 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:8200...
* connect to ::1 port 8200 from ::1 port 49848 failed: Connection refused
* Trying 127.0.0.1:8200...
* Connected to localhost (127.0.0.1) port 8200
> POST / HTTP/1.1
> Host: localhost:8200
> User-Agent: curl/8.5.0
> Accept: */*
>
< HTTP/1.1 405 Method Not Allowed
< Content-Type: application/json
< X-Content-Type-Options: nosniff
< Date: Mon, 03 Mar 2025 16:35:38 GMT
< Content-Length: 129
<
{
"error": "method not supported: this is the health check endpoint; did you mean to send data to another endpoint instead?"
}
* Connection #0 to host localhost left intact
|
Warning It looks like this PR modifies one or more |
|
Warning It looks like this PR modifies one or more |
| @@ -15,11 +15,3 @@ get: | |||
| responses: | |||
| '200': | |||
| $ref: '../components/responses/200_server_info.yaml' | |||
| post: | |||
There was a problem hiding this comment.
[to reviewer] other than openapi spec, we'll also need to remove it from obs docs in a separate PR: https://www.elastic.co/guide/en/observability/current/apm-api-info.html
There was a problem hiding this comment.
Why would you remove it? I would add a disclaimer that the / endpoint is to be used fr healthcheck or test only, but I think the use case of POST with credentials (to verify credentials) is a nice one to give our user guidance on. I would definitely clarify its purpose though.
There was a problem hiding this comment.
the intention of this very PR is to remove the "functionality" to POST to /. As such, we are removing the corresponding part in the docs. If we don't remove it in the docs, we'll be left with docs that reference an endpoint that doesn't work.
There was a problem hiding this comment.
I think the use case of POST with credentials (to verify credentials) is a nice one to give our user guidance on
I agree that we should update the docs to state that we can optionally provide credentials on the GET request to get additional info about the server. But I'm not sure if OpenAPI spec supports that.
carsonip
changed the title
/ to only accept GET and HEAD method/ to only accept GET and HEAD method
carsonip
changed the title
/ to only accept GET and HEAD method/ to only accept GET and HEAD requests
|
Warning It looks like this PR modifies one or more |
…quests (#15976) (#16016) Breaking change to change server information endpoint / to only accept GET and HEAD requests, and return 405 Method Not Allowed otherwise. This will surface any agent misconfiguration, e.g. configuring otlphttp to send to / instead of /v1/traces. (cherry picked from commit 2aaa73a) Co-authored-by: Carson Ip <carsonip@users.noreply.github.com>
Motivation/summary
Breaking change to change server information endpoint / to only accept GET and HEAD requests, and return 405 Method Not Allowed otherwise. This will surface any agent misconfiguration, e.g. configuring otlphttp to send to / instead of /v1/traces.
Checklist
POST /support docs-content#647For functional changes, consider:
How to test these changes
Confirm that GET and HEAD requests return 200, POST requests return 405.
Related issues
Fixes #15965